Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds ability for user to define additional tags for images #1388

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

fcaroline2020
Copy link

@fcaroline2020 fcaroline2020 commented Nov 14, 2024

Adds new build cli flag tag that will tag the image with the user defined tag and then push

Example CLI command
./devpod-cli build . --devcontainer-path ./devcontainer.json --force-build --debug --tag test,arm64

@fcaroline2020 fcaroline2020 changed the title Adds ability for use to define additional tags for images Adds ability for user to define additional tags for images Nov 14, 2024
@bkneis bkneis requested a review from a team November 14, 2024 14:42
Copy link
Contributor

@bkneis bkneis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution @fcaroline2020! The implementation looks good to me. Just a couple of small remarks and the lint issues to address

cmd/build.go Outdated
@@ -54,6 +54,15 @@ func NewBuildCmd(flags *flags.GlobalFlags) *cobra.Command {
}
}

// validate tags
if cmd.Tag != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cmd.Tag is initialised as []string{} so won't be nil, I think it would be better to check len(cmd.Tag) > 0

@@ -87,10 +87,27 @@ func (r *runner) Build(ctx context.Context, options provider.BuildOptions) (stri
)
}

// Setup all image tags (prebuild and any user defined tags)
ImageRefs := []string{prebuildImage}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: imageRefs over ImageRefs


// tag the image
for _, imageRef := range ImageRefs {
err = dockerDriver.TagDevContainer(ctx, prebuildImage, imageRef)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to check err is not nil or log here

if err != nil {
return "", errors.Wrap(err, "push image")
for _, imageRef := range ImageRefs {
err = dockerDriver.PushDevContainer(ctx, imageRef)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this could be a one liner with err check

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants